Improvement of context and quantization tables paragraphs #56

JeromeMartinez · 2017-04-14T14:33:09Z

No description provided.

dericed · 2017-04-14T17:33:29Z

ffv1.md


-PDF:$$context=Q_{0}[l-tl]+\left|Q_{0}\right|(Q_{1}[tl-t]+\left|Q_{1}\right|(Q_{2}[t-tr]+\left|Q_{2}\right|(Q_{3}[L-l]+\left|Q_{3}\right|Q_{4}[T-t])))$$
+PDF:$$context=Q_{0}[l-tl]+Q_{1}[tl-t]+Q_{2}[t-tr]+Q_{3}[L-l]+Q_{4}[T-t]$$


I had presumed that the pipes (|a|) were to expressed absolute values, but in this edit they are simply removed. Does this semantic meaning?

There is abs() in the current RFC part, but I don't see what is absolute value of a table, and I don't see corresponding code in FFmpeg source code.
Note that the RFC version was containing multiplication too, and I don't see it too in FFmpeg source code.
See this snipset.

I am not sure of it, help would be appreciated for confirmation.

I added the abs() in the RFC intended it to be a semantic equivalent to the pipes used in the LaTeX version. In the context of the plain text RFC, I thought abs() would be more readable than |a|. But I am not certain that the pipes in the LaTeX version indicate an absolute value but that is what the mathematical function section implies. Ping to @michaelni.

dericed · 2017-04-14T17:34:38Z

ffv1.md

@@ -191,6 +191,8 @@ Background: a two's complement signed 16-bit signed integer was used for storing

 ## Context

+For samples, compared to pixel X, named:


Suggested rewording:

Samples, positioned relative to pixel X, are referenced to with the following abbreviations:

dericed · 2017-04-14T17:38:44Z

ffv1.md

 RFC:```

-If the context is smaller than 0 then -context is used and the difference between the sample and its predicted value is encoded with a flipped sign.
+If context >= 0 then `context` is used and the difference between the sample and its predicted value is encoded as is, else `-context` is used and the difference between the sample and its predicted value is encoded with a flipped sign.


Consider tilde quotes on context >= 0.

Do I understand correctly that if context is equal to -1 then since -1 is not greater than 0, then -(-1) is used, ie 1?

Right. Contexts can be positive only, the sign of stored context seems to be used only for saying that the encoded difference must be subtracted from the predicted value instead of added.
See this snipset

dericed · 2017-04-14T17:39:53Z

ffv1.md

@@ -201,28 +203,41 @@ Background: a two's complement signed 16-bit signed integer was used for storing
 +---+---+---+---+
 ```

-The quantized sample differences L-l, l-tl, tl-t, t-T, t-tr are used as context:
+The quantized sample differences are used as context:


Consider These 5 quantized sample differences are used as context:

dericed · 2017-04-14T17:41:01Z

ffv1.md


-PDF:$$Q_{i}[a-b]=Table_{i}[(a-b)\&255]$$
+PDF:$$Q_{j}[k]=quant_tables[i][j][k\&255]$$


Meaning of j and k?

they are arbitrary values in that case, as j and k are on the left and right, and they'll be in formulas when Q is used.
In practice, j is the context and k the pixel difference.
I don't think it is need to define them here, as we see that when Q is used.

dericed · 2017-04-14T17:42:46Z

ffv1.md

+
+## Quantization table sets
+
+Several quantization table sets can be used in the same bitstream.


Can this be more specific? Is the number of quantization table sets per bitstream: >=0, >=1?
Is there a max limit to the count of quantization table?

>=1, it is quant_table_count value. maybe we can add the constraint "must not be 0" in quant_table_count definition.

FFmpeg has an hard-coded maximum of 8, but looks like this is not necessary in specs, so "unlimited" (as much as width or height i.e. at some point it is too much :) ).

a positive integer

dericed · 2017-04-14T17:44:42Z

ffv1.md

+- For Cb and Cr planes, `quant_table_index [ 1 ]` index is used
+- For Alpha plane, `quant_table_index [ (version <= 3 || chroma_planes) ? 2 : 1 ]` index is used
+
+Background: in first implementations of FFV1 bitstream, the index for Cb and Cr planes was stored even if it is not used (chroma_planes set to 0), this index is kept for version <= 3 on order to keep compatibility with bitstreams in the wild.


on order -> in order

dericed · 2017-04-14T17:46:13Z

ffv1.md

 ```c
 function                                                      | type
 --------------------------------------------------------------|-----
 QuantizationTablePerContext(i, j, scale) {                    |
    v = 0                                                     |
    for( k = 0; k < 128; ) {                                  |
-        len - 1                                               | sr
-        for( a = 0; a < len; a++ ) {                          |
+        len_minus1                                            | ur


Consider defining len_minus as a temporary counter.

it is not a counter, it is a temporary value, so not defined. It is a bit explained in the intro ("storing the number of equal entries -1"), does it need a dedicated explicit definition? as it is temporary, I suggest not to do it.

JeromeMartinez · 2017-05-18T19:54:55Z

PR updated from comments from Dave, and including wording from #64 (typo fix is not backported as the list is completely removed, in order to avoid duplicated and potentially unsynchronized content)

michaelni · 2017-05-18T22:01:30Z

This contains quite a mixture of changes, some purely cosmetic some bitstream changes. These belong to several seperate commits

dericed · 2017-05-23T01:19:22Z

ffv1.md


-## Quantization
+## Quantization tables


This section header is already used on line 1174. Duplicate section names should be avoided.

dericed · 2017-05-23T01:19:45Z

ffv1.md

@@ -218,28 +220,41 @@ Background: a two's complement signed 16-bit signed integer was used for storing
 +---+---+---+---+
 ```

-The quantized sample differences L-l, l-tl, tl-t, t-T, t-tr are used as context:
+Relative to any sample `X`, the 5 quantized sample differences are used as context:


suggesting: the following 5 quantized

JeromeMartinez · 2017-06-03T15:17:21Z

These belong to several seperate commits

Got it. Let's start with #68 then I'll update this PR.

dericed · 2017-07-01T19:04:21Z

status?

JeromeMartinez · 2017-07-02T11:19:59Z

PR updated, @michaelni and @dericed please review again.

I don't see multiplications, abs and parenthesis at this place in FFmpeg FFV1 decoder code, so I remove them from specs.
I understand the complex computing for context is already done by a pre-computing in QuantizationTable(i, j, scale) function, with multiplication of intermediate scale value, keeping multiplications, abs and parenthesis at this place would make the compute used twice instead of one.

(or... What do I miss in the FFmpeg FFV1 decoder code?)

dericed

LGTM, but admit that I don't understand this equation well. I think I had misinterpreted the LaTeX when I added the parenthesis and abs.

michaelni · 2017-07-02T13:44:05Z

LGTM, but admit that I don't understand this equation well. I think I had misinterpreted the LaTeX when I added the parenthesis and abs.

Yes, i must have missed it but the |Q| stuff was meant as the number of elements / magnitude in the quantization table not as the absolute value of something

dericed reviewed Apr 14, 2017

View reviewed changes

JeromeMartinez force-pushed the context branch 2 times, most recently from f9378fe to 040bb95 Compare May 18, 2017 19:52

dericed reviewed May 23, 2017

View reviewed changes

Context formula uses quant_table directly

5b5e42e

JeromeMartinez force-pushed the context branch from 040bb95 to 5b5e42e Compare July 2, 2017 11:11

dericed approved these changes Jul 2, 2017

View reviewed changes

michaelni merged commit 5b5e42e into FFmpeg:master Jul 2, 2017

JeromeMartinez deleted the context branch January 3, 2018 18:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvement of context and quantization tables paragraphs #56

Improvement of context and quantization tables paragraphs #56

JeromeMartinez commented Apr 14, 2017

dericed Apr 14, 2017

JeromeMartinez Apr 14, 2017

dericed Apr 14, 2017

dericed Apr 14, 2017

dericed Apr 14, 2017

JeromeMartinez Apr 14, 2017

dericed Apr 14, 2017

dericed Apr 14, 2017

JeromeMartinez Apr 14, 2017

dericed Apr 14, 2017

JeromeMartinez Apr 14, 2017

retokromer Jun 3, 2017

dericed Apr 14, 2017

dericed Apr 14, 2017

JeromeMartinez Apr 14, 2017

JeromeMartinez commented May 18, 2017

michaelni commented May 18, 2017

dericed May 23, 2017

dericed May 23, 2017

JeromeMartinez commented Jun 3, 2017

dericed commented Jul 1, 2017

JeromeMartinez commented Jul 2, 2017

dericed left a comment

michaelni commented Jul 2, 2017


		PDF:$$context=Q_{0}[l-tl]+\left\|Q_{0}\right\|(Q_{1}[tl-t]+\left\|Q_{1}\right\|(Q_{2}[t-tr]+\left\|Q_{2}\right\|(Q_{3}[L-l]+\left\|Q_{3}\right\|Q_{4}[T-t])))$$
		PDF:$$context=Q_{0}[l-tl]+Q_{1}[tl-t]+Q_{2}[t-tr]+Q_{3}[L-l]+Q_{4}[T-t]$$

		@@ -191,6 +191,8 @@ Background: a two's complement signed 16-bit signed integer was used for storing

		## Context

		For samples, compared to pixel X, named:


		PDF:$$Q_{i}[a-b]=Table_{i}[(a-b)\&255]$$
		PDF:$$Q_{j}[k]=quant_tables[i][j][k\&255]$$


		## Quantization table sets

		Several quantization table sets can be used in the same bitstream.

Improvement of context and quantization tables paragraphs #56

Improvement of context and quantization tables paragraphs #56

Conversation

JeromeMartinez commented Apr 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeMartinez commented May 18, 2017

michaelni commented May 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeMartinez commented Jun 3, 2017

dericed commented Jul 1, 2017

JeromeMartinez commented Jul 2, 2017

dericed left a comment

Choose a reason for hiding this comment

michaelni commented Jul 2, 2017